test: rename fixture config to default_config to avoid name conflict#1836
test: rename fixture config to default_config to avoid name conflict#1836bearomorphism wants to merge 1 commit intomasterfrom
Conversation
bearomorphism
requested review from
Lee-W,
noirbizarre and
woile
as code owners
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1836 +/- ##
=======================================
Coverage 97.98% 97.98%
=======================================
Files 60 60
Lines 2686 2686
=======================================
Hits 2632 2632
Misses 54 54 ☔ View full report in Codecov by Sentry. |
bearomorphism
force-pushed
the
test-config
branch
from
a2166d7 to
12e529a
Compare
bearomorphism
force-pushed
the
test-config
branch
from
12e529a to
79ce43e
Compare
bearomorphism
changed the title
There was a problem hiding this comment.
duplicated fixtures
bearomorphism
force-pushed
the
test-config
branch
from
79ce43e to
50ea789
Compare
noirbizarre
left a comment
noirbizarre
left a comment
There was a problem hiding this comment.
I think just remove the duplicate config fixture in tests/commands/conftest.py is enough.
I find config more explicit and producing shorted line that default_config (for which I would expect a non-default config somewhere).
Plus, pytest fixture pattern philosophy push to reuse names to override fixture instead of multiplicating fixture, so I think it's best to just keep config
|
Thanks for explanation, I didn't know the philosophy details behind pytest fixture. The main motivation of this PR was that we have a module called Now I agree that we should keep the name unchanged. |
|
I will close this PR and make create another one for removing duplicated fixtures. |
2 participants
No description provided.